In bug 1355351 a for loop is replaced by a while loop - for (; ancestor; ancestor = ancestor->GetParent()) { - ancestorFrame = ancestor->GetPrimaryFrame(); - if (ancestorFrame) { - break; - } + while (!ancestor->GetPrimaryFrame()) { + // FIXME(emilio): Should this use the flattened tree parent instead? + ancestor = ancestor->GetParent(); + MOZ_ASSERT(ancestor, "we can't have a display: contents subtree root!"); It also removed the null check embedded in the for loop. Coverity caught this: *** CID 1405541: Null pointer dereferences (FORWARD_NULL) /layout/base/nsCSSFrameConstructor.cpp: 8543 in nsCSSFrameConstructor::ContentRemoved(nsIContent *, nsIContent *, nsIContent *, nsCSSFrameConstructor::RemoveFlags, bool *, nsIContent **)() 8537 // Remove it once that's fixed. 8538 ClearUndisplayedContentIn(aChild, aContainer); 8539 } 8540 MOZ_ASSERT(!childFrame || !GetDisplayContentsStyleFor(aChild), 8541 "display:contents nodes shouldn't have a frame"); 8542 if (!childFrame && GetDisplayContentsStyleFor(aChild)) { >>> CID 1405541: Null pointer dereferences (FORWARD_NULL) >>> Assigning: "ancestor" = "aContainer". 8543 nsIContent* ancestor = aContainer; 8544 while (!ancestor->GetPrimaryFrame()) { 8545 // FIXME(emilio): Should this use the flattened tree parent instead? 8546 ancestor = ancestor->GetParent(); 8547 MOZ_ASSERT(ancestor, "we can't have a display: contents subtree root!"); 8548 } If aContainer can be null, we should add the null check back.

My idea is that that null check isn't needed. A display contents child always has an ancestor, given we blockify the root. It's the same that allow us to assert ancestor in the bottom of the loop. Let me assert that.

Comment on attachment 8862361 [details] Bug 1360157: Assert that a display: contents child always has a parent. https://reviewboard.mozilla.org/r/134290/#review137438

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c6537f15d2ea Assert that a display: contents child always has a parent. r=mats